Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allow-unrecognized Flag #17

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

kchason
Copy link
Owner

@kchason kchason commented Oct 3, 2023

Allow undefined CDO concepts to be exempted and not cause a failure.

  • Adds a new allow-unrecognized flag or CASE_ALLOW_UNRECOGNIZED to not fail on unrecognized CDO concepts.

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

CASE Validation Results

Summary

FileValid
./tests/data/no-context.json
./tests/data/space file.json
./tests/data/simple-case.jsonld

Details

./tests/data/no-context.json 

Validation Report

Conforms: True

./tests/data/space file.json 

Validation Report
Conforms: True

./tests/data/simple-case.jsonld 

Validation Report
Conforms: True

f,
case_version=case_version,
abort_on_first=abort_on_failure,
allow_warnings=allow_unrecognized,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line shows the current granularity potential of this configuration parameter.

case_validate.validate does not have a specific option for ignoring warnings generated by the IRI typo-checker function. It only has an option for ignoring all warnings, which also includes SHACL validation warnings.

@ajnelson-nist
Copy link
Contributor

My review boils down to a matter of maintenance opinion.

I believe, personally, that the feature introduced in this pull request is harmful, via application of the Robustness Principal: "Be conservative in what you send, and liberal in what you accept". I am, again personally, a subscriber to the spirit of RFC 9413:

Time and experience show that negative consequences to interoperability accumulate over time if implementations silently accept faulty input.

This PR introduces a feature that says it is permissible to a user to ignore warnings in specifically the IRIs in data professing to be a part of a CDO ontology or CDO-adopted ontology.

For better or worse, case-utils currently offers no such specific support. Warnings of undefined CDO IRIs are treated with the same severity as sh:Warning-severity constraint violations from SHACL validation reports: If the data is validated with case_validate --allow-warnings, all warnings are accepted(/excepted). So, in implementation, the feature in this PR has an effect that is incongruous with its keywording.

I am not personally happy with the idea of putting effort into supporting such a split in warning categories. If a graph has a typo'd IRI, I believe it should be fixed in the generating application. If a graph tried using a new custom concept under a CDO namespace, I believe it should go into some drafting namespace, rather than attempt to search CDO resources for a concept definition that it won't find, or induce pain on downstream data adopters when they can't integrate data from that faulty generator.

For the moment, I am a NACK on this PR because of the effect-scope incongruity. I'm more comfortable with this PR adding an option to expose allow_warnings via call-environment configuration. That and allow_infos are features that case-utils forwards from pyshacl, and pyshacl implements those from design induced by sh:severity, so SHACL already opened the "Here's how to ignore warnings" door. I'm not comfortable introducing a new subtle way to have faulty data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants